-
Notifications
You must be signed in to change notification settings - Fork 26
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore: migrate to crypto/ecdh #178
Conversation
Signed-off-by: Shiwei Zhang <[email protected]>
The CI tests are failing since |
Signed-off-by: Shiwei Zhang <[email protected]>
I would suggest reviewing and merging PR after the |
Seems like this PR is on a good track, once CI is passing, and our test coverage is recovered, this seems like a great improvement |
@shizhMSFT, with 1.22 released, are you ready to finalize this PR? |
I'm not sold about this solution. IMO, we should either remove support for ECDSA curves without public component, or assume that we have to call a deprecated function. |
I agree with that.
I'm afraid of being scanned as a vulnerability by some scanners in the future although it is not.
Can you elaborate more on this? |
Unless you are parsing a key that is inside a signature or encryption, developers can translate keys with compressed points to ones with uncompressed points. Any library that doesn't support compressed points will need to throw if it cannot translate, because operations will fail that required both points. |
Thanks for the elaboration.
Do we want to go that way? |
Based on the above, we're going to close this PR and open a new issue for guidance. |
Resolves #168 by migrating to
crypto/ecdh
.Here are notable changes:
crypto/ecdh
is introduced ingo 1.21
, the minimum go version is thus bumped to1.21
from1.18
.crypto/ecdh
does not expose low-level elliptic curve operations, custom curves are no longer supported. Thus the related test case is removed. /cc @qmuntal